feat(power-pages): add security skills - #151

…23950)

Sync 3 upstream commits from microsoft/power-platform-skills:
- Power Pages ALM: 10 new skills, 45 lib helpers, 6 reference docs (microsoft#167)
- Power Pages security: manage-firewall, manage-headers, scan-site, security-review (microsoft#151)
- Docs: update Power Pages skill list (microsoft#174)

163 files added, 16 modified. Equinor guardrails preserved:
- SECURITY.md, marketplace.json, code-apps/, .devcontainer/ unchanged
- Review record updated (power-pages v1.3.0 → v2.0.0)
- All review records pass schema validation

…#170)

* Apply Approval Gate pattern to non-ALM power-pages skills

Extend the catalog in references/approval-gates.md (§6.13-§6.24)
to cover the 12 non-ALM skills: create-site, deploy-site,
add-server-logic, add-cloud-flow, setup-auth, integrate-webapi,
setup-datamodel, add-sample-data, add-seo, create-webroles,
audit-permissions, integrate-backend. 45 gates + 9 not-a-gates
inserted as HTML comment markers + human 🚦 blocks above each
AskUserQuestion call.

Flip lint to hard-fail across the whole plugin -- the ALM-only
warn-only branch in scripts/lint-skills-alm.js has been removed.
severityForSkill() now returns 'error' for every skill. Update
the corresponding test in scripts/tests/lint-skills-alm.test.js.

AGENTS.md Key Patterns generalized: the Approval Gate convention
applies plugin-wide. New skills must extend the catalog in the
same PR that introduces an AskUserQuestion, or CI blocks the PR.

Workflow files get header comments explaining why power-pages-
alm-lint and power-pages-script-tests are deliberately separate
(policy enforcement vs functional tests; single-OS fast-fail vs
cross-OS matrix).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Catalog the 4 security skills picked up during rebase

Rebase onto origin/main brought in PR #151 which added 4 new
power-pages skills: manage-firewall, manage-headers, scan-site,
security-review. With the warn-only lint branch removed in the
previous commit, those skills now had unmarked AskUserQuestion
prompts and broke CI.

These skills use a different prompt shape — most calls happen
inside a runtime "recommend then ask" loop described in prose,
not at statically-locatable call sites. The catalog now has a
new §6.24a section explaining the convention for runtime-loop
skills: meta-mention sections get not-a-gate markers; concrete
call sites get full gate markers.

Catalog rows added:
- §6.25 manage-firewall — 1 not-a-gate (option-rules-meta)
- §6.26 manage-headers — 1 gate (per-finding loop, plan)
- §6.27 scan-site — 1 not-a-gate (option-rules-meta)
- §6.28 security-review — 2 gates (2.1 goal, 5.3 next-action)

Total: 3 gates + 2 not-a-gates. Lint passes (0 findings).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Apply review fixes: doc, list-rendering, lint cleanups, ALM orphan markers

Addresses the 12 findings from the pre-PR code review. Fixes ordered by
severity:

MUST FIX:
- PLUGIN_DEVELOPMENT_GUIDE.md: drop the stale "non-ALM warn-only" text
  on line 273 and mirror the v3 hard-fail language. AGENTS.md tells new
  authors to read this file first, so the stale doc would have misled
  them.

MARKDOWN RENDERING:
- audit-permissions, create-site (4 places), integrate-webapi: move
  gate marker blocks ABOVE the ordered list intro sentence so they
  don't sit between numbered items. CommonMark/GFM split a list at an
  unindented HTML comment + blockquote, restarting numbering. The
  rendered numbering on GitHub was 1,2,3,1,2 instead of 1..N.

PRE-EXISTING FILE BUGS:
- Stray ``` token at add-cloud-flow:346 and integrate-backend:271
  (both from PR #167) — removed. These broke fence parity for
  everything below them.

LINT CLEANUPS:
- severityForSkill() is now a constant (SKILL_SEVERITY); unused
  skillName parameter and void no-op removed; call sites simplified.
- Dead warnings-only exit branch in main() removed — no code path
  produces severity 'warning' post-v3.
- Tightened checkSectionPairing: m < promptLine (strictly before),
  not m <= promptLine, so a marker on the SAME line as the prompt no
  longer trivially satisfies the rule.
- CATALOG_GATE_ID_PATTERN now case-insensitive [A-Za-z] to align
  with GATE_MARKER_PATTERN — future CamelCase / underscored skill
  names won't break GATE-must-be-in-catalog.

ALM CATALOG ORPHANS:
- 16 catalog gate rows previously lacked SKILL.md markers (introduced
  pre-v3). Resolved in two ways:
  - Added markers where the prompt call site is concrete:
    test-site:5.5.form-submit, setup-pipeline:4.3.name-conflict +
    6b.v2-migration, export-solution:2.identify + 3.overwrite,
    force-link-environment:2.host-url + 2.dev-env, ensure-pipelines-
    host:3.C.host-type + 3.C.env-pick + 4.0.pre-call + 4.A.pre-call
    + 4.sandbox-confirm + 4.C.ppac-done, plan-alm:2.q4-host (renamed
    from q4-stage-env to match the actual host-selection prompt).
  - Removed orphan rows that described gates not implemented as
    separate AskUserQuestion calls: deploy-pipeline:6.1.pac-fallback-
    consent (covered by 6.0.final-consent prose), configure-env-
    variables:6.confirm-matrix (covered by 2.selection's per-stage
    matrix), force-link-environment:2.host-fallback + 2.dev-fallback
    (redundant — the gate IDs above cover the fallback path).

PROSE ANCHORS:
- setup-auth Phase 1.4 prompt prose now includes the literal
  backticked AskUserQuestion + colon anchor required by
  PROMPT_LINE_PATTERN. Removing the gate marker would now actually
  trigger GATE-must-have-marker.

VERIFIED: lint reports 0 findings; 984/984 plugin tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Address PR review feedback: lint upgrades + sweeping refinements

Reverts an accidental scope expansion, adds two new lint rules, closes
the runtime-loop coverage hole, renames the awkward 'Blast radius if
skipped' field, and removes dead module-level code. All 9 feedback
items from the pre-PR review are addressed.

LINT:
- CATALOG_GATE_ID_PATTERN: revert underscore that was silently added —
  was [A-Za-z0-9_-]*, now [A-Za-z0-9-]* matching the original kebab-only
  intent. The case-insensitivity change to [A-Za-z] (from [a-z]) is
  kept, with a comment explaining the asymmetric grammar with
  GATE_MARKER_PATTERN.
- New rule: CATALOG-row-must-have-marker — reverse check (catalog row →
  SKILL.md marker). Catches the orphan-row class of bug v3 closed by
  hand. Parses §6 catalog tables for rows tagged 'gate' and verifies
  each ID has a matching <!-- gate: ID --> marker in some SKILL.md.
- New rule: GATE-prose-block-required — every gate marker must be
  followed within 10 lines by a line carrying the 🚦 sentinel. Catches
  prose-block deletion without forcing structural rewrite of 80+ legacy
  v2 single-line markers.
- Tightened m < promptLine (strict precede) with a new test asserting
  marker-on-prompt-line fails the rule.
- Removed dead ALM_SKILLS export + its membership test — no downstream
  consumer existed.
- Simplified SKILL_SEVERITY comment (the wishful 'future per-skill
  policy' framing).

CATALOG:
- §3.2 plan-vs-consent: added a paragraph explaining why deploy-dispatch
  prompts ('Deploy now?') are tagged plan when the destructive consent
  lives inside the dispatched skill's own gate. Resolves the
  create-site:8.deploy (plan) vs deploy-site:3.confirm-env (consent)
  apparent inconsistency.
- §6.4 / §6.7 / §6.9: removed archaeological commentary about merged /
  removed rows. Git blame has it.
- §6.24a: rewritten to reflect that the runtime-loop coverage hole is
  now CLOSED — manage-firewall and scan-site got real call-site
  anchors.
- §10: added cross-plugin lint TODO for shared/skills/report-issue/;
  expanded the v3 PR changelog with the new lint rules + field rename.
- §11: added phase-number-drift convention (any SKILL.md phase renumber
  must grep the catalog), runtime-loop coverage caveat (now mostly
  resolved), and lint-prose-block weakness (only 🚦 enforced, not the
  3 structured labels).

RUNTIME-LOOP ANCHORS (F13 — closes the coverage hole):
- manage-firewall: new gate manage-firewall:3.action-choice (plan, at
  '### Default approach') + manage-firewall:3.execute-consent (consent,
  at '### Plan-validate-execute'). The destructive WAF mutations
  (enable/disable/add/update/delete rule) are now lint-anchored.
- scan-site: new gate scan-site:3.action-choice (plan, at '### Default
  approach'). The scan-trigger decision is now lint-anchored.
- Both prose blocks updated to include the literal 'AskUserQuestion:'
  string that PROMPT_LINE_PATTERN requires.

FIELD RENAME (F14):
- 'Blast radius if skipped:' → 'Why we ask:' across all 60 prose blocks
  + §4.1 catalog template. The old label suggested the gate's purpose
  was to describe Cancel state; the actual content explains why the
  gate prompts in the first place.

VERIFIED: lint reports 0 findings (with new reverse + prose-block
rules active); 984/984 plugin tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Address PR review feedback round 2: lint robustness + test coverage + doc consistency

Addresses 10 review-skill findings + 9 Copilot inline comments + the
Copilot Autofix on the rule-count wording.

## Lint script (scripts/lint-skills-alm.js)

LINT REGEX:
- CATALOG_GATE_ROW_PATTERN now tolerates up to 3 leading spaces before the
  pipe (GFM table indentation tolerance). Was `^\|`, now `^\s{0,3}\|`.
  Without this, a future markdown reformat that nests §6 tables under a
  parent list silently disables orphan detection for those rows.

GATE-prose-block-required:
- Window now INCLUDES the marker's own line (slice(startIdx, startIdx+10),
  was slice(startIdx+1, startIdx+11)). Single-line compact-style markers
  with marker + 🚦 on one line now pass.
- The 🚦 search now skips code-fence regions. Was a plain `.includes('🚦')`
  per line; now pre-computes per-line fence state across the full file
  and ignores 🚦 inside ```...``` blocks. Closes the "future contributor
  deletes the real Gate prose block but leaves an example 🚦 in a bash
  example within the window" silent-pass case.

CATALOG-row-must-have-marker:
- Docstring updated. Previously claimed "Waivable: yes — inline
  <!-- alm-lint-ignore: ... --> in the catalog row's section". The
  implementation never called extractIgnores on the catalog file, so
  inline-ignore never worked. New docstring says .almlintignore allowlist
  is the only suppression mechanism (the rule operates on the catalog as
  a whole, not per SKILL.md, so per-row inline ignore would be confusing
  to model). Matches actual behavior.

CLEANUPS:
- Removed dead `skillNameFromFile()` function. No callers since
  severityForSkill was deleted; not in module.exports.
- Removed `SKILL_SEVERITY` constant. Half-applied (2 of 9 severity:
  sites used it) — inlined `'error'` everywhere for consistency.
- Updated Usage header comment to match v3 exit-code behavior.
- main() now concatenates findings into one string + single
  process.stderr.write, then sets process.exitCode instead of calling
  process.exit. Prevents stderr-truncation on Windows when piping to a
  log file with large finding counts.

## Tests (scripts/tests/lint-skills-alm.test.js)

Added 7 new tests for the two rules introduced in commit 8d02547 that
shipped with zero coverage:

CATALOG-row-must-have-marker:
- fires when a catalog gate row has no SKILL.md marker
- passes when every gate row has a marker
- skips not-a-gate rows (no marker required for them)
- tolerates leading whitespace on table rows (GFM compliance)

GATE-prose-block-required:
- fires when marker has no 🚦 within 10 lines
- passes when 🚦 sentinel follows within window
- ignores 🚦 inside a fenced code block
- tolerates 🚦 on the same line as the marker (single-line style)

Test count: 45 → 52 (+7). Plugin-wide: 984 → 992.

## Docs

approval-gates.md / AGENTS.md / PLUGIN_DEVELOPMENT_GUIDE.md:
- Updated "five GATE rules" → "seven gate-related rules" (Copilot's
  finding). Lists all 7 enforced rules now. Includes the v3 additions
  CATALOG-row-must-have-marker and GATE-prose-block-required.

approval-gates.md §9 Decisions row 4:
- Marked the v2 "ALM: hard-fail. Non-ALM: warn-only" decision as
  superseded by §10. The §10 history correctly captures the v2→v3
  transition but §9 read as a current recommendation in isolation.

Verified: lint 0 findings; 992/992 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Fix readSettingsFile to handle the keyed-object stages shape

Discovered while validating PR #170 against the real Citizens portal site
(C:\Projects\Citizens portal). The site's deployment-settings.json uses
the Microsoft-standard 2024 schema shape:

  {
    "$schema": "https://schemas.microsoft.com/power-platform/deployment-settings/2024",
    "stages": {
      "Deploy to Staging": {
        "EnvironmentVariables": [...]
      }
    }
  }

— a keyed OBJECT of stages. readSettingsFile only handled two shapes:
1. Top-level `EnvironmentVariables: []` (single-stage)
2. `Stages: []` array with `{ Name, EnvironmentVariables }` entries

The keyed-object shape returned 0 entries, which meant
validate-deployment-settings.js silently passed even on known-broken
values like `@KeyVault(vaultName=...;secretName=...)`.

Real-world evidence: the Citizens portal's docs/alm/last-deploy.json
records a failed deploy attempt (2026-05-21) where deployment-settings.json
contained `@KeyVault(...)` for c311_api_secret. The pre-deploy validator
"validation passed (validation does not check Secret reference format)"
— because the parser couldn't read the file. The deploy then waited ~4h
in the host queue before failing with `ImportAsHolding failed: The value
provided as a secret reference does not match a valid secret reference
format`. This is exactly what the v3 deploy-pipeline:7.6.4.strip-secret-
values gate is supposed to prevent — but the gate's underlying validator
was broken.

Changes:

- readSettingsFile now handles all three shapes. Each returned entry
  now carries `stageLabel` (null for shape 1, the stage name for shapes
  2 and 3), so downstream consumers can attribute findings to the right
  stage without re-parsing the file.

- validate-deployment-settings.js had a duplicated read-settings parser
  (readEntriesPreservingStage) with the same bug. Deleted — it now
  uses readSettingsFile directly.

- Tests:
  - Existing readSettingsFile tests updated to expect the new stageLabel
    field on entries.
  - New test: keyed-object shape (Microsoft 2024 schema). Validates the
    real-world fix against a fixture matching the Citizens portal file.
  - validate-deployment-settings.test.js renamed readEntriesPreservingStage
    references to readSettingsFile.

Verified end-to-end against the real Citizens portal file:

  node validate-deployment-settings.js --settingsFile <citizens-portal>
  → summary { invalid: 1 }, one finding flagging the @KeyVault(...) value
    on c311_api_secret as 'kv-placeholder' / 'invalid'.

Plugin tests: 993/993 (was 992; +1 new shape-3 test).
Lint: 0 findings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Fix two readSettingsFile regressions surfaced by Copilot review

1. Dedupe loss in validateSettings → readSettingsFile path

   readSettingsFile dedupes by schemaName when no stageLabel filter is
   provided. This is correct for callers like verify-env-var-values
   that want a single "configured value of X" per schema. But it is
   WRONG for validate-deployment-settings, which must inspect every
   stage's value independently:

     - Staging: c311_api_secret = "https://kv.../secrets/api-secret"  (valid)
     - Production: c311_api_secret = "@KeyVault(vaultName=...)"       (invalid)

   Pre-fix, dedupe kept Staging's value and Production's broken value
   was silently skipped. validateSettings would report invalid: 0
   even though the deploy would fail at ImportAsHolding on Production.

   Fix: added a `preserveAllStages` option to readSettingsFile.
   validate-deployment-settings now calls it with that flag set so
   every per-stage entry is inspected.

2. Mixed-case `Stages` key when value is an object

   Comment said "either casing of stages/Stages key" but code only
   checked `parsed.stages || parsed.STAGES`. A hand-authored file
   using `Stages: { ... }` (capital-S + object form — falls through
   shape 2's array check because the value isn't an array) returned 0
   entries.

   Fix: added `parsed.Stages` to the fallback chain. All three casings
   (`stages` / `Stages` / `STAGES`) now resolve to the object-shape
   path when the value is a plain object.

3. Regression tests (+3, total 996/996)

   - Mixed-case `Stages` object is read correctly.
   - readSettingsFile dedupe (default) vs preserveAllStages behavior
     pinned with concrete expectations.
   - End-to-end: validateSettings catches a Production-only invalid
     value even when Staging is valid (the dedupe-loss scenario).

Verified against real Citizens portal site — still correctly catches
the @KeyVault(...) placeholder on c311_api_secret.

Lint: 0 findings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>